Skip to content

Conversation

@BlairCurrey
Copy link
Contributor

@BlairCurrey BlairCurrey commented Nov 21, 2025

Parent branch for grant spent amounts calculation rework. Most of the code was already reviewed, but I did add something here that was not captured in the other issues. And that is getting the grant spent amounts in this edge case:

  1. amount has been spent against grant
  2. new rafiki version is deployed with this method for calculating spent amounts
  3. GET /outgoing-payment-grant` is called (no spent amounts, but amount was spent against the grant)

In this case we are summing all historical payments (added that in this PR). We do the same thing on the outgoing payment create side so I shared the logic.

fixes #3369
linear https://linear.app/interledger/issue/RAF-1031/implement-new-grant-spent-amount-calculation

BlairCurrey and others added 7 commits April 10, 2025 14:29
* feat: add grant spent amount table

* fix: change interval start/end string to timestamp

* feat: fix filename, update op grant table

* fix: rm erroneously added migration to tsconfig
* feat: calculate grant spent amounts from new table

* chore: linting

* fix: revise grant spent amounts calc to track interval

* fix: throw better error, dont inti interval amounts unless they exist

* fix: linter, wip commented out code

* refactor(backend): rename interval statuses

* fix: ensure correct amount (interval/total) is used for grant spent amount

* test(backend): grant spent amounts

* fix(backend): make logic more typesafe, avoid having to defend w/ internal server error

* test(backend): legacy grant calculations

* test(backend): classify payment interval fn

* test(backend): rm commented-out classify payment interval case

* refactor(backend): improve semantics of interval helper fn

* test(backend): add non-interval cases to create op

* Apply suggestions from code review

Co-authored-by: Max Kurapov <[email protected]>

* refactor: simplifiy interval calc

* chore: rm unused imports

* refactor: validate grant payment logic

* refactor: improve grant spent amount calc type safety

* refactor: improve grant spent amount calc type safety

* fix: missing trx

* chore: handle bad interval state

* chore: rm some superfluous comments

* chore: format

---------

Co-authored-by: Max Kurapov <[email protected]>
)

* feat: calculate grant spent amounts from new table

* chore: linting

* fix: revise grant spent amounts calc to track interval

* fix: throw better error, dont inti interval amounts unless they exist

* fix: linter, wip commented out code

* refactor(backend): rename interval statuses

* fix: ensure correct amount (interval/total) is used for grant spent amount

* test(backend): grant spent amounts

* fix(backend): make logic more typesafe, avoid having to defend w/ internal server error

* test(backend): legacy grant calculations

* test(backend): classify payment interval fn

* test(backend): rm commented-out classify payment interval case

* refactor(backend): improve semantics of interval helper fn

* test(backend): add non-interval cases to create op

* Apply suggestions from code review

Co-authored-by: Max Kurapov <[email protected]>

* feat: partially handled grant spent amount scenarios on settle

* feat: handle non-interval partial settleamount cases

* test(backend): failure grant spent amount case for sucessive payment

* test(backend): some grant calc interval cases

* test(backend): counting grant payments across interval boundaries

* chore(backend): rm commented out test

* test(backend): new grant spent amount on payment completion

* test(backend): improve interval grant calc test to show summation

* test(backend): failure edge case

* chore(backend): rm comment

* chore(backend): fix lint errors

* chore(backend): rm debug logs

* test(backend): fix failing

* Update packages/backend/src/open_payments/payment/outgoing/lifecycle.ts

Co-authored-by: Max Kurapov <[email protected]>

* fix(backend): rm extra spent amount record check

* feat(backend): return debit amount from .pay

* test(backend): failing test for grant spent amount bug

- if 2 payments are create then 2 payments are processed,
it assocaites the wrong spent amount with the wrong payment

* fix(backend): grant spent amounts calc race conditions

- partially implemented fix (missing interval stuff),
not fully validated by tests yet

* test(backend): grant calc race condition

* fix(backend): grant spent amount race conditions with failure

* chore(backend): format

* test(backend): add failing interval race condition test

* fix(backend): interval boundary/race condition edge case

- edge case is when there are create/complete race conditions around interval boundaries

* test(backend): improve edge case test

- ensures we are testing that the latest interval amount is
used as the base for interval amounts, not just the payment
in the interval being completed

* fix(backend): dont query for latest interval payment unecunnecessarily

- not necessary if we are within the interval of the payment being completed

* fix(backend): type mismatch

* test(backend): payment retries do not add additional spent record

* fix(backend): use correct debit
amount for spent amount recalc

* fix(backend): set correct payment state on
grant spent amount updates

* refactor(backend): dedupe revert/handle grant spent amounts

- turned duplicated logic into shared functions
- example fn where revert/update is all in one,
but felt it was more complicated

* chore: rm unused fn

* test(backend): fix ilp pay return expectations

* refactor(backend): move revert/update grant spent amounts to op service

* chore: rm unused import

* fix: handle grant spent amounts on payment cancellation

* chore: cleanup commented out code

* chore: rm submodule added in error

* chore(backend): add error logs

* fix(backend): explicit grant spent amount formation, more test assertions

* refactor(backend): only return receive amt from .pay

* fix(backend): build error

---------

Co-authored-by: Max Kurapov <[email protected]>
@netlify
Copy link

netlify bot commented Nov 21, 2025

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit fabae8d
🔍 Latest deploy log https://app.netlify.com/projects/brilliant-pasca-3e80ec/deploys/69682aa2e608a000082a997e

@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. type: source Changes business logic labels Nov 21, 2025
@BlairCurrey BlairCurrey changed the title Bc/raf 1031/grant spent amounts feat: grant spent amounts Nov 21, 2025
@github-actions
Copy link

github-actions bot commented Nov 21, 2025

🚀 Performance Test Results

Test Configuration:

  • VUs: 4
  • Duration: 1m0s

Test Metrics:

  • Requests/s: 44.16
  • Iterations/s: 14.74
  • Failed Requests: 0.00% (0 of 2655)
📜 Logs

> [email protected] run-tests:testenv /home/runner/work/rafiki/rafiki/test/performance
> ./scripts/run-tests.sh -e test "-k" "-q" "--vus" "4" "--duration" "1m"

Cloud Nine GraphQL API is up: http://localhost:3101/graphql
Cloud Nine Wallet Address is up: http://localhost:3100/
Happy Life Bank Address is up: http://localhost:4100/
cloud-nine-wallet-test-backend already set
cloud-nine-wallet-test-auth already set
happy-life-bank-test-backend already set
happy-life-bank-test-auth already set
     data_received..................: 958 kB 16 kB/s
     data_sent......................: 2.0 MB 34 kB/s
     http_req_blocked...............: avg=7.67µs   min=2.05µs   med=4.84µs   max=1.84ms   p(90)=6.06µs   p(95)=6.68µs  
     http_req_connecting............: avg=441ns    min=0s       med=0s       max=421.77µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=89.96ms  min=6.77ms   med=73.47ms  max=639.93ms p(90)=157.2ms  p(95)=177.44ms
       { expected_response:true }...: avg=89.96ms  min=6.77ms   med=73.47ms  max=639.93ms p(90)=157.2ms  p(95)=177.44ms
     http_req_failed................: 0.00%  ✓ 0         ✗ 2655
     http_req_receiving.............: avg=85.06µs  min=27.58µs  med=71.85µs  max=2.44ms   p(90)=109.11µs p(95)=135.08µs
     http_req_sending...............: avg=35.97µs  min=9.51µs   med=26.75µs  max=2.93ms   p(90)=37.86µs  p(95)=53.48µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=89.84ms  min=6.62ms   med=73.39ms  max=639.86ms p(90)=157.13ms p(95)=177.32ms
     http_reqs......................: 2655   44.16369/s
     iteration_duration.............: avg=271.19ms min=156.95ms med=257.19ms max=1.14s    p(90)=333.99ms p(95)=365.55ms
     iterations.....................: 886    14.737864/s
     vus............................: 4      min=4       max=4 
     vus_max........................: 4      min=4       max=4 

BlairCurrey and others added 4 commits November 24, 2025 13:26
* feat: add /GET outgoing-payment-grant route

* fix: change null state return to return null

* fix: rm old comments

* fix: add get grant spent amount service tests

* fix: add get grant spent amount controller tests

* fix(backend): linter

* fix(backend): tenanted path

* fix: dont require identifier in outgoing payment access item introspection

* feat(backend): rm unecessary check

* test(auth): add test

* test(backend): add middleware test

* refactor(backend): cleanup grant spent amount retrieval

* chore: fix lint

* test(backend): fix grant id check

* test(backend): simplify logic, return 0 amount when able

* refactor(backend): token introspection middleware

- make 2nd introspection middleware for
new route instead of generalizing exisitng middleware

* test(backend): rm no longer needed test

* fix: rm unused import

* Update packages/backend/src/open_payments/auth/middleware.ts

Co-authored-by: Max Kurapov <[email protected]>

* fix: return 403 for non create grant for spent amounts

used to return null spent amounts

* fix: add logs, use luxon interval method

* fix: scope of else block

* Update packages/backend/src/open_payments/auth/middleware.ts

Co-authored-by: Max Kurapov <[email protected]>

* chore: fix name

* feat: test for new middleware/shared fn

* chore: make action required

---------

Co-authored-by: Max Kurapov <[email protected]>
@github-actions github-actions bot added the pkg: auth Changes in the GNAP auth package. label Dec 11, 2025
@BlairCurrey BlairCurrey marked this pull request as ready for review December 16, 2025 03:03
mkurapov
mkurapov previously approved these changes Jan 5, 2026
Copy link
Contributor

@mkurapov mkurapov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After we have updated the NodeJS client, we should also update an integration test to verify grant spent amounts as well


await middleware(ctx, next)

expect(mockTokenIntrospectionClient.introspect).toHaveBeenCalledWith({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also validate that next was called heere

njlie
njlie previously approved these changes Jan 9, 2026
Copy link
Contributor

@njlie njlie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, seemed to work when I tested the local env a bit

})
.then(() => {
return knex.raw(
'CREATE INDEX outgoingPaymentGrantSpentAmounts_grantId_createdAt_desc_idx ON "outgoingPaymentGrantSpentAmounts" ("grantId", "createdAt" DESC)'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll leave it for now.... but I was thinking about changing this:

'CREATE INDEX outgoingPaymentGrantSpentAmounts_grantId_createdAt_desc_idx ON "outgoingPaymentGrantSpentAmounts" ("grantId", "createdAt" DESC)'

To include the interval fields:

knex.raw('CREATE INDEX ... ON "outgoingPaymentGrantSpentAmounts" ("grantId", "intervalStart", "intervalEnd", "createdAt" DESC)')`

because we look it up like this in the worker when processing the payment:

await OutgoingPaymentGrantSpentAmounts.query(deps.knex)
          .where('grantId', grantId)
          .where('intervalStart', latestPaymentSpentAmounts.intervalStart)
          .where('intervalEnd', latestPaymentSpentAmounts.intervalEnd)
          .orderBy('createdAt', 'desc')
          .first()

Dont have great intuition on tradeoffs. I guess it would slow writes some amount in all cases and speed up reads some amount in a subset of cases (when we lookup by interval - only in worker I think). Probably no db storage space difference since a created_at index is going to already lead to a unique index entry per row. Overall I guess it feels like a bad idea. I guess we probably read a bit more (read and write on create, read and write in worker, read in GET /outgoing-payment-grant) but we only lookup by interval in one place.

@BlairCurrey
Copy link
Contributor Author

Made a few small tweaks:

  • recorded metric for those cases where we log an error due to unexpected state but dont through
  • fixed a few spots where we cast from BigInt -> Number -> BigInt to use Math.max but lost precision in doing so

sanducb
sanducb previously approved these changes Jan 14, 2026
@github-actions github-actions bot removed the type: ci Changes to the CI label Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: auth Changes in the GNAP auth package. pkg: backend Changes in the backend package. type: source Changes business logic type: tests Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement new grant spent amount calculation

5 participants